Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ Refactor/#423 ] group page query key 리팩토링 #424

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

ljh0608
Copy link
Member

@ljh0608 ljh0608 commented Aug 27, 2024

✨ 해당 이슈 번호 ✨

#423

todo

  • query key - query key factory로 마이그레이션
  • 해당 query key들을 사용하는 invalidQueries 부분 수정

📌 내가 알게 된 부분

자세한 부분이 더 궁금하시면 해당 링크를 확인해주세요!

원래의 mile 서비스는 다음과 같이 각 페이지 폴더 내에 커스텀훅을 모아두는 페이지 상단에 객체로 키를 정의해두었습니다.

“커스텀훅을 사용해서 관심사를 분리하고, 객체 키를 사용해서 오타나 중복 등의 휴먼에러를 줄인다.” 라는 접근으로 키를 구성하였지만 한가지 놓친 부분은 쿼리키의 hierarchy 구조를 잘 활용하지 못하는 방향으로 구성되어있었습니다. 이로인해 쿼리키를 invalid할때 관련된 여러 쿼리를 invalid하는데에 어려움이 있고, 다른 페이지에서 사용하는 쿼리키는 여전히 중복될 수 있다는 문제를 가지고 있었습니다.

query key 중복을 막기 위해 전역적으로 key를 관리해야하는가? no!
두괄식으로 말하자면 Query key를 /src/utils/queryKeys.ts 에 전역적으로 저장하는 것은 권장하는 방법이 아닙니다. 이는 공동 배치(colocation)의 이점을 잃는 것입니다.
키를 전역으로 관리했을 때 아래와 같은 부분에서 문제가 발생할 수 있습니다. (자세한 내용은 노션 링크를 첨부하겠습니다.)

  • 유지 관리성
  • 적용 가능성
  • 사용 편의성

제가 query key 리팩토링을 고려한 부분은 아래와 같습니다. 아래 4개의 고려사항을 생각하며 query key factories를 사용해서 문제를 해결해나가보겠습니다.

  1. 중복되지 않는 키
  2. 배열로 위계가 잡힌 키 (효율적인 invalid를 위해)
  3. 쉽게 파악할 수 있는 키 구조
  4. 공동배치 (colocation)
image

위 이미지와 같이 쿼리키 위계를 잡아 아래와 같이 리팩토링하여 키를 관리합니다. 이러한 방법은@lukemorales/query-key-factory 라는 라이브러리를 사용해서도 구현할 수 있지만 key의 위계를 확실히하고 중복을 막자 라는 현재 목적에 맞게 라이브러리 사용없이 구현했습니다. 이는 추후 라이브러리의 장단점을 조금 확인해보고 도입할 예정입니다!

export const groupQueryKey = {
  all: ['group'],
  info: () => [...groupQueryKey.all, 'info'],
  detail: (groupId: string) => [...groupQueryKey.info(), 'detail', groupId],
  auth: (groupId: string) => [...groupQueryKey.info(), 'auth', groupId],
  isPublic: (groupId: string) => [...groupQueryKey.info(), 'public', groupId],
  topics: (groupId: string) => [...groupQueryKey.all, 'topic', groupId],
  topic: (groupId: string) => [...groupQueryKey.topics(groupId), 'todayTopic'],
  posts: (topicId: string, groupId: string) => [...groupQueryKey.topics(groupId), 'posts', topicId],
  curiousPosts: (groupId: string) => [...groupQueryKey.info(), 'curiousPosts', groupId],
  curiousWriters: (groupId: string) => [...groupQueryKey.info(), 'curiousWriter', groupId],

  user: () => [...groupQueryKey.all, 'user'],
  userName: (groupId: string) => [...groupQueryKey.user(), 'userName', groupId],
  userInfo: (writerNameId: number | undefined) => [
    ...groupQueryKey.user(),
    'userInfo',
    writerNameId,
  ],
  userGroups: () => [...groupQueryKey.user(), 'groups'],
};

📌 질문할 부분

Q. 왜 일반적인 배열 형태가 아닌 [...groupQueryKey.all, 'info'] 함수형식 () => [...groupQueryKey.all, 'info']으로 선언하나요?
A. 일반적으로 groupQueryKey를 정의하는 객체 내에서 ...groupQueryKey.all에 접근할 수 없기 때문이다.

위와 같이 함수형식으로 정의하고 함수가 실행될 때는 groupQueryKey가 이미 정의된 상태이므로 에러가 발생하지 않는다. (객체 내부 속성값으로 함수를 선언하면 함수는 정의시점이 아닌 실행시점에 평가됨)

Q. as const를 왜 사용해야하나요?
A. const assertion을 사용해서 보다 단순 문자열이 아닌 구체적인 타입을 지정하게 할 수 있습니다.

객체나 배열에서는 읽기 전용(read only)로 변경하고 타입의 범위를 좁힐 수 있습니다.
이러한 특징으로 현재의 쿼리키들이 단순히 string 배열이 아닌 각자의 타입의 범위로 좁힐 수 있습니다.

📌스크린샷(선택)

image

@ljh0608 ljh0608 added ♻️ Refactor 코드 리팩토링 재훈 labels Aug 27, 2024
@ljh0608 ljh0608 self-assigned this Aug 27, 2024
Copy link

vercel bot commented Aug 27, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
mile-client ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 27, 2024 5:48am

@github-actions github-actions bot added the size/m size/m label Aug 27, 2024
Copy link
Contributor

@moondda moondda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Member

@namdaeun namdaeun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

query key에 대해 별다른 생각을 하지 않고 작성하고 있던 저 스스로를 돌아볼 수 있었던 아티클이었슴니다... 👍🏻 재훈님이 작성해주신 아티클을 꾸준히 참고해보며 앞으로 체계적으로 query key를 관리할 수 있는 사람이 되어보겠슴니다 ..

Query Key Factory에 대해서도 처음 알게됐는데, 함수와 쿼리키에 대한 로직을 분리할 수 있다는 점에서도 좋아보이네요

많이 배워갑니다 :)

Copy link
Contributor

@se0jinYoon se0jinYoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm
근데 궁금한 점은, 현재의 위계 설정이 추후에 바뀌게 될 가능성은 일단 고려하지 않고 구현되어있는건가요? 현재까지의 저희 서비스, 그리고 로직상에서 두고 본다면 위계와 중복을 최대로 줄여서 효율적으로 쿼리키와 캐싱을 관리한다고 생각되는데, 이러한 위계가 추후에 변경될 가능성에 대해선 어떻게 생각하시는지 궁금합니다!
쿼리키에 대해 늘 듣기만 했지 제대로 공부해본 적은 없는데 덕분에 또 인사이트 얻어가네요 감사합니다 수고하셨어요!!

@ljh0608
Copy link
Member Author

ljh0608 commented Sep 9, 2024

lgtm 근데 궁금한 점은, 현재의 위계 설정이 추후에 바뀌게 될 가능성은 일단 고려하지 않고 구현되어있는건가요? 현재까지의 저희 서비스, 그리고 로직상에서 두고 본다면 위계와 중복을 최대로 줄여서 효율적으로 쿼리키와 캐싱을 관리한다고 생각되는데, 이러한 위계가 추후에 변경될 가능성에 대해선 어떻게 생각하시는지 궁금합니다! 쿼리키에 대해 늘 듣기만 했지 제대로 공부해본 적은 없는데 덕분에 또 인사이트 얻어가네요 감사합니다 수고하셨어요!!

좋은 질문 감사합니다!

현재까지의 저희 서비스, 그리고 로직상에서 두고 본다면 위계와 중복을 최대로 줄여서 효율적으로 쿼리키와 캐싱을 관리한다고 생각되는데,
이 부분이 이전 쿼리키 관리를 말씀하시는건지 제가 지금 변경한 것에 대해 말씀하시는 건지 잘 모르겠어서 다시 한번 답변주시거나 회의 때 얘기해보면 좋을 것 같습니다.

위계가 추후에 변경되는 가능성에 대한 제 답변은 일단 "변경될 가능성이 크진 않다." 입니다. 커뮤니티 서비스에서 글모임 > 글 > 글내용, 댓글, 좋아요 정도로 큰 틀의 위계는 변경되지 않을 것이라고 생각했어요. 결국은 포함관계에 있는 데이터니까요. 또한 제가 설계한 방식이 쿼리키 변경, 삭제, 추가에 대해서 기존의 방식보다 조금 더 용이하다고 생각이 들었습니다! 위계를 삭제할 땐 다른 상위키를 연결해주면 되고 추가할 땐 그저 바로 위에 사용되고 있는 쿼리키를 연결해주면 되니까요!

그리고 이런 고민이 들 때마다 저는 YAGNI 개발원칙을 떠올리는데요. You Ain't Gonna Need it 이라는 말로 그 의미는 현재는 사용하지 않지만 확장성을 고려해서 미리 작업하지말라. 필요한 작업만 해라 입니다. 제 생각은 이런데 이부분 어떻게 생각하시나요??

@se0jinYoon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
♻️ Refactor 코드 리팩토링 size/m size/m 재훈
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

4 participants